Add tests to see if pre_model_hook works for tests around snowflake_warehouse #1070
Conversation
|
running test without the |
dbt/adapters/snowflake/impl.py
Outdated
| logger.info("Running pre_model_hook:") | ||
| logger.info(f"Default warehouse: {default_warehouse}, Selected warehouse: {warehouse}") | ||
| if warehouse == default_warehouse or warehouse is None: | ||
| return None | ||
| previous = self._get_warehouse() | ||
| self._use_warehouse(warehouse) | ||
| logger.info(f"Changed wareho use from {previous} to {warehouse}") |
There was a problem hiding this comment.
Do we want to put these logs behind condition of warehouses being different?
There was a problem hiding this comment.
@McKnight-42 I don't think we should add any new logging here. The actual queries being run (select current_warehouse() as warehouse + use warehouse different_warehouse) are already appearing in the logs.
dbt/adapters/snowflake/impl.py
Outdated
| logger.info("Running pre_model_hook:") | ||
| logger.info(f"Default warehouse: {default_warehouse}, Selected warehouse: {warehouse}") | ||
| if warehouse == default_warehouse or warehouse is None: | ||
| return None | ||
| previous = self._get_warehouse() | ||
| self._use_warehouse(warehouse) | ||
| logger.info(f"Changed wareho use from {previous} to {warehouse}") |
There was a problem hiding this comment.
@McKnight-42 I don't think we should add any new logging here. The actual queries being run (select current_warehouse() as warehouse + use warehouse different_warehouse) are already appearing in the logs.
| "config-version": 2, | ||
| "models": { | ||
| "test": { | ||
| "snowflake_warehouse": "DBT_TEST_DOES_NOT_EXIST", |
There was a problem hiding this comment.
Yes, nicely done - we should expect the dbt test to try using a Snowflake warehouse that doesn't exist (use warehouse DBT_TEST_DOES_NOT_EXIST), where previously it wouldn't, and so it should fail with Object does not exist, or operation cannot be performed.
There was a problem hiding this comment.
went ahead and also added a assert to catch if that error message stops showing up.
There was a problem hiding this comment.
For the sake of completeness, can we add a positive test case that uses this config option, just to make sure it's setting it? The error message we're getting in the negative test case is pretty general.
There was a problem hiding this comment.
added a new test
McKnight-42
changed the title
|
|
||
|
|
||
| class TestValidConfigWarehouse: | ||
| @pytest.fixture(scope="class") |
There was a problem hiding this comment.
I think you just need to define this and the other model in outside of all classes once.
|
This is just a test I don't think it is a feature. |
currently labeled as such because it ties with the core prs dbt-labs/dbt-core#10258 and dbt-labs/dbt-core#10245 but yeah could possibly just be a under the hood update. |
McKnight-42
changed the title
resolves #1059
docs dbt-labs/docs.getdbt.com/#
Problem
We want to be able to correctly handle arbitrary configs in tests correctly, for models this is currently done with
pre/post_model_hookand for snowflake this is mainly for swappingsnowflake_wareshousevalues, currently we don't actually recognize this config in during tests. by adding updatingdbt-coremethods we have opened up the ability to pass these into tests correctly but must make specific modifications in the adapters themselves.asks:
Solution
we can use AdapterLogger to add loggger.info as we need in the
model_hookmethodsadd a new tests to handle scenarios i.e we update
snowflake_warehouseto a invalid warehouse.blocked by:
dbt-labs/dbt-core#10258
Checklist